Skip to content

ENH: Add cibuildwheel and automated PyPI release - #66

Merged
jnolan14 merged 17 commits into
freesurfer:masterfrom
larsoner:cbw
Jan 8, 2026
Merged

ENH: Add cibuildwheel and automated PyPI release#66
jnolan14 merged 17 commits into
freesurfer:masterfrom
larsoner:cbw

Conversation

@larsoner

@larsoner larsoner commented Oct 6, 2025

Copy link
Copy Markdown
Contributor

Closes #64
Closes #17

Seems to be working:

https://github.com/larsoner/surfa/actions/runs/18286055605

Also enables PyPI automated publishing when a tagged release is cut

Comment thread pyproject.toml Outdated
@@ -1,2 +1,7 @@
[build-system]
requires = ['setuptools', 'wheel', 'Cython>=3.0', 'numpy']
requires = ['setuptools', 'wheel', 'Cython>=3.0', 'numpy>=2']

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest updating the python min to 3.10 at the same time (3.9 is EOL Oct 30), happy to push that. Building with NumPy >= 2 is useful to ensure that code is both NumPy 1.x and 2.x compatible. But I can roll this back if needed and just make sure that cibuildwheel installs a NumPy >= 2, though it does complicate the code a bit.

@larsoner larsoner mentioned this pull request Oct 6, 2025
@larsoner

larsoner commented Oct 6, 2025

Copy link
Copy Markdown
Contributor Author

I also just saw #54... after this PR is merged, if someone bumps the version in __init__.py and creates a release on GitHub with a tag like v0.6.4 then it should land in PyPI.

@larsoner

larsoner commented Nov 26, 2025

Copy link
Copy Markdown
Contributor Author

Modified to use abi3 builds for compat with future versions of Python

EDIT: Appears to have worked: https://github.com/larsoner/surfa/actions/runs/19711633336/job/56473558969

@larsoner

Copy link
Copy Markdown
Contributor Author

Rebased and green here 🚀

jnolan14 and others added 8 commits December 22, 2025 13:09
Updated OS runners
Dropped duplicate macos-14 and macos-14-large (this one requires payment so we can live without the intel tests for now)
…rkflow to only trigger on tagged release. Updated os matrix to match test workflow.
@jnolan14

jnolan14 commented Jan 8, 2026

Copy link
Copy Markdown
Collaborator

The actions file for building the wheels and modernizing the pyprojecet.toml is greatly appreciated!
Unfortunately, we still need to support python 3.8 and numpy v1, at least for the time being. We should be dropping that in the relatively near future, at which point, I'll enable that workflow and we'll start publishing wheels on PyPI.

@jnolan14
jnolan14 merged commit 5a168aa into freesurfer:master Jan 8, 2026
20 checks passed
@larsoner

larsoner commented Jan 8, 2026

Copy link
Copy Markdown
Contributor Author

Unfortunately, we still need to support python 3.8 and numpy v1, at least for the time being.

FWIW I wouldn't let that stop you from publishing wheels for the platforms enabled above. Having some usable wheels out there is better than none IMO.

Also, wheels compiled using NumPy 2 should work with NumPy 1.0, so any of the wheels here should be usable with NumPy 1.x already. I can probably fairly easily modify the build to support Python 3.8 and 3.9 etc. I'll see if it's easy enough...

@larsoner
larsoner deleted the cbw branch January 8, 2026 18:00
@larsoner

larsoner commented Jan 8, 2026

Copy link
Copy Markdown
Contributor Author

Hmmm from the pushes the setup here now does some unexpected stuff... some PR discussion probably would have been useful rather than pushing commits I think. For example:

  1. There isn't really a reason to run cibuildwheel on both 22.04 and 24.04 because these both use docker under the hood to build and test wheels, so should be identical (testing both really tests docker's ability to run on these two systems rather than anything about surfa). Thus my original preference for running on ubuntu-latest (lower maintenance burden than bumping ubuntu versions every once in a while).
  2. If you want to try different platforms, best practice is to build wheels on one, then -- if you want! -- set up a test_wheels job that downloads the artifact and tests on whatever platforms you want, like ubuntu-22.04 and ubuntu-24.04 separately.
  3. ☝️ Similar things would also be more useful for macos-* variants as well -- those build with a MACOSX_DEPLOYMENT_TARGET set to the same value (10.x or 11 depending on arch) for both 14 and 15, but at least for those the testing phase will differ a little bit instead of being identical. (But still, you'll end up with what should be identical wheel builds for both -14 and -15, and then if you download the artifacts one will overwrite the other which is... less than ideal.)
  4. The ubuntu-24.04-arm support from 0564746 was removed for some reason. It's very fast (usually faster than the Intel runs!) so I'm not sure why it was removed?
  5. As above, NumPy 1.x should be supported anyway just by having been built against 2.x, so no need for the "once numpy 1 support is dropped" comment at the top
  6. The on: pr was removed. If this is because of potential CI slowdowns, it's really easy for me to add something that looks for [wheels] in the commit message for example and only build wheels in PRs that have that in the latest commit message (similar to what I did for h5py who faced a slowdown). This would have shown for example whether or not the cp{38,39...} addition worked etc. As it stands now you can't see it...

I'm happy to open a PR to restore some of this stuff if it helps, and happy to discuss / clarify more if it helps!

@larsoner

larsoner commented Jan 8, 2026

Copy link
Copy Markdown
Contributor Author

On and one more last, hopefully minor point -- locally running

CIBW_BUILD="cp38-*" cibuildwheel .

fails, so I expect CIs to fail. It actually fails in the testing step because it tries to build SciPy from source for the musllinux wheel. So I'd suggest either skipping testing for that wheel or -- probably better -- skip building a wheel for 3.8 musl altogether (i.e., add it to skip). If there is no SciPy wheel for a given platform, I wouldn't bother making one for surfa...

@larsoner larsoner mentioned this pull request Jan 8, 2026
@jnolan14

jnolan14 commented Jan 9, 2026

Copy link
Copy Markdown
Collaborator

Happy to discuss the changes further, should have started there.

I'd like to hold off on distributing the wheels until we can drop numpy 1.X support. You're absolutely right that it should work with the backwards compatibility in the ABI, however since equivalent behavior isn't a guarantee I'd prefer to hold off on distributing them for now.

  1. Yes, I agree, my naive thinking was 'the build and test matrices should match', but since they'll produce identical results, that's redundant
  2. I agree, this seems like a better approach
  3. Yes, my understanding is that for MacOS they will build against the system libraries, so would/could be worth testing, but as you say overwriting the artifacts is less than ideal
  4. Happy to include ARM distributions of Ubuntu if the runners are supported. Freesurfer is not officially supported on ARM Linux distributions, but that doesn't mean we can't build surfa wheels for it (same goes for Windows)
  5. As I said above, I agree, but I am quite wary of possible edge cases causing divergent behavior. I'd be satisfied after doing some more testing to ensure that this wouldn't be an issue
  6. That was the reason for removing that section. In practice, we wouldn't cut a release/tag if the tests were failing so that's probably redundant to have done.

Last message seems entirely reasonable to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Distribute binary wheels? Install on windows

2 participants